feat(rust): advance state in post commit #2396
Conversation
|
@ion-elgreco I've looked a this quick a bit and feel like a deep refactor is required for this. IMO if the commit logic is going to be responsible for providing the committed then it must return return a DeltaTableState / Snapshot. What are the expected scenarios where that struct will be |
|
@Blajda Yeah it felt a bit strange to not be able to take ownership of the snapshot. While working on it I didn't understand why the incoming snapshot could be None until I hit the create path, which obviously has no table snapshot. We should just return always a snapshot. If the incoming snapshot is None, we can just create a snapshot after committing and return that. Do you agree? |
ion-elgreco
force-pushed
the
feat/advance_state_in_post_commit
branch
from
52be0c2 to
8024cf8
Compare
ion-elgreco
force-pushed
the
feat/advance_state_in_post_commit
branch
2 times, most recently
from
7b4950e to
be2a388
Compare
ion-elgreco
force-pushed
the
feat/advance_state_in_post_commit
branch
from
be2a388 to
4544772
Compare
ion-elgreco
force-pushed
the
feat/advance_state_in_post_commit
branch
from
4544772 to
32a62c2
Compare
Description
We advance the state in the post commit now, so it's done in a single location as per suggestion from @Blajda here: #2391 (comment)
This PR also supersedes this one: #2280
Related Issue(s)